Skip to content

Conversation

@rado17
Copy link
Contributor

@rado17 rado17 commented Oct 31, 2025

Brings in P2P support.

@rado17 rado17 requested review from a team as code owners October 31, 2025 10:02
@NordicBuilder NordicBuilder added doc-required PR must not be merged without tech writer approval. manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Oct 31, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Oct 31, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
nrfxlib nrfconnect/sdk-nrfxlib@db86ee8 nrfconnect/sdk-nrfxlib#1934 nrfconnect/sdk-nrfxlib#1934/files
zephyr nrfconnect/sdk-zephyr@71ebd2b (main) nrfconnect/sdk-zephyr#3452 nrfconnect/sdk-zephyr#3452/files

DNM label due to: 2 projects with PR revision

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Oct 31, 2025

CI Information

To view the history of this post, click the 'edited' button above
Build number: 20

Inputs:

Sources:

sdk-nrf: PR head: 8e82484e1c4c34ebdf7632b7af8c589176e53fce
nrfxlib: PR head: 48bc6f9aabfbf5a5a952895f22588e8872c49aef
hostap: PR head: 528eb2d95e35c7c9b187a15d2fb7f6e5bb983181
zephyr: PR head: b473916ab499dd9a71bec83fc54c0793ee6240b9

more details

sdk-nrf:

PR head: 8e82484e1c4c34ebdf7632b7af8c589176e53fce
merge base: 7a1a161f4ecccd45ba10a5a11a33558c487473e0
target head (main): 78f8c43b230ef210d5882b222ee8e0b5960cb904
Diff

nrfxlib:

PR head: 48bc6f9aabfbf5a5a952895f22588e8872c49aef
merge base: db86ee83d38f53800bfc25da5d819548fca1ae47
target head (main): 693f0f13e743664a19f64e504a02377d9718b534
Diff

hostap:

PR head: 528eb2d95e35c7c9b187a15d2fb7f6e5bb983181
merge base: 6086dea5ee7406e1eede7f2ca6dff1b00b0f04e2
Diff

zephyr:

PR head: b473916ab499dd9a71bec83fc54c0793ee6240b9
merge base: 71ebd2b14110f5d9fa3cd23873df0d25c276611b
target head (main): dc05376c170fc2739a4881629caf49c40d485a9e
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (78)
doc
│  ├── nrf
│  │  ├── app_dev
│  │  │  ├── device_guides
│  │  │  │  ├── nrf70
│  │  │  │  │  │ wifi_advanced_security_modes.rst
│  │  ├── protocols
│  │  │  ├── wifi
│  │  │  │  ├── index.rst
│  │  │  │  │ wifi_direct.rst
│  │  ├── releases_and_maturity
│  │  │  ├── releases
│  │  │  │  │ release-notes-changelog.rst
modules
│  ├── lib
│  │  ├── hostap
│  │  │  ├── src
│  │  │  │  ├── crypto
│  │  │  │  │  │ crypto_mbedtls_alt.c
│  │  │  │  ├── drivers
│  │  │  │  │  ├── driver_zephyr.c
│  │  │  │  │  │ driver_zephyr.h
nrfxlib
│  ├── nrf_modem
│  │  ├── doc
│  │  │  │ CHANGELOG.rst
│  │  ├── include
│  │  │  │ nrf_socket.h
│  │  ├── lib
│  │  │  ├── cellular
│  │  │  │  ├── nrf9120
│  │  │  │  │  ├── hard-float
│  │  │  │  │  │  ├── libmodem.a
│  │  │  │  │  │  │ libmodem_log.a
│  │  │  │  │  ├── soft-float
│  │  │  │  │  │  ├── libmodem.a
│  │  │  │  │  │  │ libmodem_log.a
│  │  │  │  ├── nrf9160
│  │  │  │  │  ├── hard-float
│  │  │  │  │  │  ├── libmodem.a
│  │  │  │  │  │  │ libmodem_log.a
│  │  │  │  │  ├── soft-float
│  │  │  │  │  │  ├── libmodem.a
│  │  │  │  │  │  │ libmodem_log.a
│  │  │  │  ├── nrf9230
│  │  │  │  │  ├── hard-float
│  │  │  │  │  │  ├── libmodem.a
│  │  │  │  │  │  │ libmodem_log.a
│  │  │  │  │  ├── soft-float
│  │  │  │  │  │  ├── libmodem.a
│  │  │  │  │  │  │ libmodem_log.a
│  │  │  ├── dect_phy
│  │  │  │  ├── nrf9120
│  │  │  │  │  ├── hard-float
│  │  │  │  │  │  ├── libmodem.a
│  │  │  │  │  │  │ libmodem_log.a
│  │  │  │  │  ├── soft-float
│  │  │  │  │  │  ├── libmodem.a
│  │  │  │  │  │  │ libmodem_log.a
│  ├── nrf_wifi
│  │  ├── bin
│  │  │  ├── ncs
│  │  │  │  ├── default
│  │  │  │  │  │ nrf70.bin
│  │  │  │  ├── offloaded_raw_tx
│  │  │  │  │  │ nrf70.bin
│  │  │  │  ├── radio_test
│  │  │  │  │  │ nrf70.bin
│  │  │  │  ├── scan_only
│  │  │  │  │  │ nrf70.bin
│  │  │  │  ├── system_with_raw
│  │  │  │  │  │ nrf70.bin
│  │  │  ├── zephyr
│  │  │  │  ├── default
│  │  │  │  │  │ nrf70.bin
│  │  │  │  ├── offloaded_raw_tx
│  │  │  │  │  │ nrf70.bin
│  │  │  │  ├── radio_test
│  │  │  │  │  │ nrf70.bin
│  │  │  │  ├── scan_only
│  │  │  │  │  │ nrf70.bin
│  │  │  │  ├── system_with_raw
│  │  │  │  │  │ nrf70.bin
samples
│  ├── wifi
│  │  ├── wfa_qt_app
│  │  │  ├── overlay-enterprise.conf
│  │  │  │ sample.yaml
snippets
│  ├── wifi-p2p
│  │  ├── snippet.yml
│  │  │ wifi-p2p.conf
subsys
│  ├── net
│  │  ├── lib
│  │  │  ├── hostap_crypto
│  │  │  │  ├── CMakeLists.txt
│  │  │  │  ├── Kconfig
│  │  │  │  │ wpa3_psa.c
west.yml
zephyr
│  ├── doc
│  │  ├── connectivity
│  │  │  ├── networking
│  │  │  │  ├── api
│  │  │  │  │  │ wifi.rst
│  │  ├── releases
│  │  │  ├── migration-guide-4.3.rst
│  │  │  │ release-notes-4.4.rst
│  ├── drivers
│  │  ├── wifi
│  │  │  ├── nrf_wifi
│  │  │  │  ├── Kconfig.nrfwifi
│  │  │  │  ├── inc
│  │  │  │  │  ├── fmac_main.h
│  │  │  │  │  │ wpa_supp_if.h
│  │  │  │  ├── src
│  │  │  │  │  ├── fmac_main.c
│  │  │  │  │  ├── net_if.c
│  │  │  │  │  ├── wifi_mgmt.c
│  │  │  │  │  │ wpa_supp_if.c
│  ├── include
│  │  ├── zephyr
│  │  │  ├── bluetooth
│  │  │  │  ├── audio
│  │  │  │  │  │ bap.h
│  │  │  ├── net
│  │  │  │  │ wifi_mgmt.h
│  ├── modules
│  │  ├── hostap
│  │  │  ├── CMakeLists.txt
│  │  │  ├── Kconfig
│  │  │  ├── src
│  │  │  │  ├── supp_api.c
│  │  │  │  ├── supp_api.h
│  │  │  │  ├── supp_events.c
│  │  │  │  ├── supp_events.h
│  │  │  │  │ supp_main.c
│  ├── subsys
│  │  ├── bluetooth
│  │  │  ├── audio
│  │  │  │  ├── bap_broadcast_sink.c
│  │  │  │  ├── bap_scan_delegator.c
│  │  │  │  │ bap_unicast_client.c
│  │  ├── net
│  │  │  ├── l2
│  │  │  │  ├── wifi
│  │  │  │  │  ├── Kconfig
│  │  │  │  │  ├── wifi_mgmt.c
│  │  │  │  │  │ wifi_shell.c
│  ├── tests
│  │  ├── bluetooth
│  │  │  ├── tester
│  │  │  │  ├── src
│  │  │  │  │  ├── audio
│  │  │  │  │  │  ├── btp
│  │  │  │  │  │  │  ├── btp_ascs.h
│  │  │  │  │  │  │  │ btp_bap.h
│  │  │  │  │  │  ├── btp_bap.c
│  │  │  │  │  │  ├── btp_bap_broadcast.c
│  │  │  │  │  │  ├── btp_bap_broadcast.h
│  │  │  │  │  │  │ btp_bap_unicast.c
│  │  ├── bsim
│  │  │  ├── bluetooth
│  │  │  │  ├── audio
│  │  │  │  │  ├── src
│  │  │  │  │  │  ├── bap_broadcast_assistant_test.c
│  │  │  │  │  │  │ bap_scan_delegator_test.c
│  │  │  │  ├── tester
│  │  │  │  │  ├── src
│  │  │  │  │  │  │ bsim_btp.c
│  │ west.yml

Outputs:

Toolchain

Version: 4e36fbc961
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:4e36fbc961_5ea73affbf

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister - Skipped: Skipping Build & Test as it succeeded in a previous run: 19
  • ✅ Integration tests
    • ✅ test-sdk-audio
    • ✅ test_ble_nrf_config
    • ✅ test-fw-nrfconnect-ble_samples
    • ✅ test-fw-nrfconnect-chip - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nfc - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_libmodem-nrf - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_zephyr_lwm2m - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_samples - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_lwm2m - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_thingy91 - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-rs - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-fem - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-thread-main - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-find-my - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf_lrcs_positioning - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-wifi
    • ✅ test-low-level - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-dfu - Skipped: Job was skipped as it succeeded in a previous run
    • ⚠️ test-fw-nrfconnect-nrf-iot_cloud
    • ⚠️ test-fw-nrfconnect-nrf_lrcs_mosh
    • ⚠️ test-fw-nrfconnect-nrf_lrcs_positioning
Disabled integration tests
    • desktop52_verification
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-tfm
    • test-sdk-mcuboot
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

CONFIG_WIFI_NM_WPA_SUPPLICANT_P2P=y
CONFIG_WPA_CLI=y
CONFIG_WIFI_NM_WPA_SUPPLICANT_LOG_LEVEL_INF=y
CONFIG_LTO=y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment as to why this is needed.

Copilot AI review requested due to automatic review settings November 3, 2025 09:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@richabp
Copy link
Contributor

richabp commented Nov 3, 2025

There is no doc build, please check.

Copy link
Contributor

@richabp richabp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a Changelog entry for both the docs.

@rado17 rado17 requested a review from divyagona November 4, 2025 12:27
CONFIG_WIFI_NM_WPA_SUPPLICANT_P2P=y
CONFIG_WPA_CLI=y
CONFIG_WIFI_NM_WPA_SUPPLICANT_LOG_LEVEL_INF=y
CONFIG_LTO=y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and why is this needed? Also why is the log level changed?

Copilot finished reviewing on behalf of krish2718 November 26, 2025 21:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 13 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

if HOSTAP_CRYPTO_ALT_PSA
# PSA doesn't work with WPA3 builtin (uses bignum)
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "PSA doesn't work with WPA3 builtin (uses bignum)" could be misleading since this PR introduces WPA3 PSA support. Consider updating the comment to clarify:

# PSA doesn't work with WPA3 builtin implementation (uses bignum)
# Use HOSTAP_CRYPTO_WPA3_PSA for WPA3 support with PSA
Suggested change
# PSA doesn't work with WPA3 builtin (uses bignum)
# PSA doesn't work with WPA3 builtin implementation (uses bignum)
# Use HOSTAP_CRYPTO_WPA3_PSA for WPA3 support with PSA

Copilot uses AI. Check for mistakes.
@@ -16,8 +16,9 @@ tests:
sysbuild: true
build_only: true
extra_args:
- EXTRA_CONF_FILE:STRING="overlay-netusb.conf;overlay-enterprise.conf"
- EXTRA_CONF_FILE="overlay-netusb.conf;overlay-enterprise.conf"
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of :STRING= from the EXTRA_CONF_FILE line changes the CMake variable type specification. This could potentially affect how the variable is processed. Ensure this change is intentional and that the build system correctly handles the untyped variable assignment.

Suggested change
- EXTRA_CONF_FILE="overlay-netusb.conf;overlay-enterprise.conf"
- EXTRA_CONF_FILE:STRING="overlay-netusb.conf;overlay-enterprise.conf"

Copilot uses AI. Check for mistakes.
Comment on lines 929 to 930
wpa_printf(MSG_DEBUG, "WPA3-PSA: PT derivation - Group 0 (auto-select) - using "
"group 19 (NIST P-256)");
group = 19;
} else {
wpa_printf(MSG_DEBUG,
"WPA3-PSA: Unsupported group %d for PT derivation (only group 19 NIST "
"P-256 supported)",
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling logic in wpa3_psa_derive_pt_group is incorrect. When group is not 0, the function immediately returns NULL for any non-zero group value, even when group == 19 (which should be the only supported value).

The logic should be:

if (group == 0) {
    wpa_printf(MSG_DEBUG, "WPA3-PSA: PT derivation - Group 0 (auto-select) - using group 19 (NIST P-256)");
    group = 19;
} else if (group != 19) {
    wpa_printf(MSG_DEBUG, "WPA3-PSA: Unsupported group %d for PT derivation (only group 19 NIST P-256 supported)", group);
    return NULL;
}
Suggested change
wpa_printf(MSG_DEBUG, "WPA3-PSA: PT derivation - Group 0 (auto-select) - using "
"group 19 (NIST P-256)");
group = 19;
} else {
wpa_printf(MSG_DEBUG,
"WPA3-PSA: Unsupported group %d for PT derivation (only group 19 NIST "
"P-256 supported)",
wpa_printf(MSG_DEBUG, "WPA3-PSA: PT derivation - Group 0 (auto-select) - using group 19 (NIST P-256)");
group = 19;
} else if (group != 19) {
wpa_printf(MSG_DEBUG,
"WPA3-PSA: Unsupported group %d for PT derivation (only group 19 NIST P-256 supported)",

Copilot uses AI. Check for mistakes.
if (!pt) {
return;
}

/* Free PT structure */
os_free(pt);
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak: If the PT structure is successfully allocated but there is a next PT in the chain, the function sae_deinit_pt only frees the first PT structure and doesn't traverse the linked list. This will leak memory for any additional PT structures in the chain.

The function should recursively free all PT structures in the linked list:

void sae_deinit_pt(struct sae_pt *pt)
{
    struct sae_pt *next;
    
    while (pt) {
        next = pt->next;
        os_free(pt);
        pt = next;
    }
}
Suggested change
if (!pt) {
return;
}
/* Free PT structure */
os_free(pt);
struct sae_pt *next;
while (pt) {
next = pt->next;
os_free(pt);
pt = next;
}

Copilot uses AI. Check for mistakes.
Comment on lines 1480 to 1485
diff = value[i] - modulus[i] - borrow;
if (diff > value[i] || (value[i] == modulus[i] && borrow)) {
borrow = 1;
} else {
borrow = 0;
}
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect borrow detection logic. The condition (value[i] == modulus[i] && borrow) is checking if values are equal AND there's a borrow, but the borrow flag should be set when the subtraction underflows. The correct logic should check if the subtraction result is greater than the original value (indicating underflow):

diff = (uint32_t)value[i] - (uint32_t)modulus[i] - borrow;
borrow = (diff > 0xFF) ? 1 : 0;
value[i] = diff & 0xFF;

The current implementation may incorrectly set the borrow flag.

Suggested change
diff = value[i] - modulus[i] - borrow;
if (diff > value[i] || (value[i] == modulus[i] && borrow)) {
borrow = 1;
} else {
borrow = 0;
}
diff = (uint32_t)value[i] - (uint32_t)modulus[i] - borrow;
borrow = (diff > 0xFF) ? 1 : 0;

Copilot uses AI. Check for mistakes.
- wfa_qt_app_SNIPPET="wifi-enterprise"
- CONFIG_NRF_WIFI_DATA_HEAP_SIZE=90000
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Configuration conflict: CONFIG_NRF_WIFI_DATA_HEAP_SIZE is set to 80000 in overlay-enterprise.conf (line 1) but overridden to 90000 in sample.yaml (line 21). This creates confusion about which value should be used. Consider either:

  1. Removing the setting from one location
  2. Using a single consistent value
  3. Documenting why different values are needed in different contexts
Suggested change
- CONFIG_NRF_WIFI_DATA_HEAP_SIZE=90000

Copilot uses AI. Check for mistakes.
@@ -64,7 +64,7 @@ manifest:
# https://developer.nordicsemi.com/nRF_Connect_SDK/doc/latest/zephyr/guides/modules.html
- name: zephyr
repo-path: sdk-zephyr
revision: f316ea6f7080c448f423f68b44a0f6fe78d7f455
revision: pull/3452/head
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using pull/3452/head as a revision is not a valid permanent reference for production code. Pull request references are temporary and will become invalid once the PR is merged or closed. This should be changed to a specific commit SHA or a stable branch/tag reference.

Replace with a concrete commit SHA:

revision: <commit_sha>
Suggested change
revision: pull/3452/head
revision: <commit_sha>

Copilot uses AI. Check for mistakes.
Comment on lines 199 to 200
if (group == 0) {
group = 19;
} else {
wpa_printf(MSG_ERROR, "WPA3-PSA: Only group 19 (NIST P-256) is supported, got %d",
group);
return -1;
}
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling logic in sae_set_group is incorrect. When group is not 0, the function immediately returns an error for any non-zero group value, even when group == 19 (which should be the only supported value).

The logic should be:

if (group == 0) {
    group = 19;
} else if (group != 19) {
    wpa_printf(MSG_ERROR, "WPA3-PSA: Only group 19 (NIST P-256) is supported, got %d", group);
    return -1;
}

Copilot uses AI. Check for mistakes.
Comment on lines +393 to +389
/* SAE commit message should be exactly 98 bytes: [group][scalar][element] */
if (len != 98) {
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The buffer size validation uses a magic number 98 without explanation. This should be defined as a named constant to improve code maintainability and clarity. Consider defining:

#define WPA3_SAE_COMMIT_MESSAGE_LEN 98  /* [group(2)][scalar(32)][element(64)] */

Then use this constant throughout the code instead of the hardcoded value.

Copilot uses AI. Check for mistakes.
Comment on lines 1390 to 1391
os_memcpy(local_scalar, op->local_commit + 2, 32);
os_memcpy(peer_scalar, op->peer_commit + 2, 32);
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic numbers 32 for NIST P-256 scalar/field sizes and buffer sizes appear throughout the code without named constants. This reduces maintainability. Consider defining constants such as:

#define WPA3_SAE_P256_SCALAR_LEN 32
#define WPA3_SAE_P256_ELEMENT_X_LEN 32
#define WPA3_SAE_P256_ELEMENT_Y_LEN 32

This would make the code more self-documenting and easier to maintain.

Copilot uses AI. Check for mistakes.
rado17 and others added 4 commits November 27, 2025 03:37
Pull Wi-Fi P2P support.

Signed-off-by: Ravi Dondaputi <[email protected]>
Pull in changes done to support WPS connection and allowing
probe request to be sent to supplicant.

Signed-off-by: Ravi Dondaputi <[email protected]>
Create snippet for P2P build.

Signed-off-by: Ravi Dondaputi <[email protected]>
This implements the PSA variant of WPA3, disables the internal WPA3 in
the supplicant and uses the nRF Oberon's WPA3 HKDF APIs.

Signed-off-by: Chaitanya Tata <[email protected]>
kapbh and others added 4 commits November 27, 2025 03:38
Add Wi-Fi direct (p2p mode) document.

Signed-off-by: Kapil Bhatt <[email protected]>
Remove the type, else local twister is ignoring the overlays.

Signed-off-by: Chaitanya Tata <[email protected]>
Enable LTO to fix the overflow and also to make room for future
increase.

Signed-off-by: Chaitanya Tata <[email protected]>
Add an entry for WPA3-SAE PSA experimental support.

Signed-off-by: Chaitanya Tata <[email protected]>
Copilot AI review requested due to automatic review settings November 26, 2025 22:09
Copilot finished reviewing on behalf of krish2718 November 26, 2025 22:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 12 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* - Group 19 (NIST P-256) only
* - H2E (Hash-to-Element) support
* - HNP (Hash-and-Password) support
* - No GDH (Group-Dependent Hash) support
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "No GDH (Group-Dependent Hash) support" but the code includes GDH support logic (see lines 996-1002 with PSA_ALG_WPA3_SAE_GDH). This documentation is inconsistent with the implementation.

Suggested change
* - No GDH (Group-Dependent Hash) support
* - GDH (Group-Dependent Hash) support

Copilot uses AI. Check for mistakes.

wpa_printf(MSG_DEBUG, "WPA3-PSA: Derive PT - group %d (PSA only)", group);

if (ssid_len > 32) {
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hard-coded value 32 for SSID length validation should be defined as a named constant (e.g., WPA3_PSA_MAX_SSID_LEN) for better maintainability, especially since it's also used at line 55 for the ssid array size.

Copilot uses AI. Check for mistakes.
static int wpa3_psa_derive_commit(struct wpa3_psa_operation *op)
{
psa_status_t status;
u8 commit_data[1024]; /* Buffer for commit message */
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coded buffer size 1024 is used for commit_data without justification. This should be defined as a named constant, ideally reusing the already defined WPA3_PSA_MAX_COMMIT_LEN constant from line 31.

Suggested change
u8 commit_data[1024]; /* Buffer for commit message */
u8 commit_data[WPA3_PSA_MAX_COMMIT_LEN]; /* Buffer for commit message */

Copilot uses AI. Check for mistakes.
static int wpa3_psa_process_commit(struct wpa3_psa_operation *op, const u8 *commit_data,
size_t commit_len)
{
psa_status_t status;
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coded send_confirm_counter value {0x01, 0x00} appears to be little-endian representation of 1, but this is not documented. Add a comment explaining the byte order and why this specific encoding is required by the protocol.

Suggested change
psa_status_t status;
psa_status_t status;
/* The send_confirm_counter is encoded as a 2-byte little-endian value.
* {0x01, 0x00} represents the integer 1 in little-endian format, as required
* by the WPA3 SAE protocol and the PSA PAKE API for the confirm counter field.
*/

Copilot uses AI. Check for mistakes.
Comment on lines +1362 to +1373
pmk_buf = (uint8_t *)os_malloc(32);
if (!pmk_buf) {
wpa_printf(MSG_ERROR, "WPA3-PSA: Failed to allocate memory for PMK");
goto cleanup;
}

status = psa_export_key(shared_key_id, pmk_buf, 32, &pmk_len);
if (status != PSA_SUCCESS) {
wpa_printf(MSG_ERROR, "WPA3-PSA: Failed to export shared key (PMK): %d", status);
goto cleanup;
}

Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory allocation uses os_malloc(32) with a hard-coded size. This should use a named constant representing the expected PMK size, or better yet, use the size from PSA APIs to avoid assumptions about the key size.

Suggested change
pmk_buf = (uint8_t *)os_malloc(32);
if (!pmk_buf) {
wpa_printf(MSG_ERROR, "WPA3-PSA: Failed to allocate memory for PMK");
goto cleanup;
}
status = psa_export_key(shared_key_id, pmk_buf, 32, &pmk_len);
if (status != PSA_SUCCESS) {
wpa_printf(MSG_ERROR, "WPA3-PSA: Failed to export shared key (PMK): %d", status);
goto cleanup;
}
{
psa_key_attributes_t key_attrs = PSA_KEY_ATTRIBUTES_INIT;
size_t pmk_buf_size = 0;
status = psa_get_key_attributes(shared_key_id, &key_attrs);
if (status != PSA_SUCCESS) {
wpa_printf(MSG_ERROR, "WPA3-PSA: Failed to get key attributes: %d", status);
goto cleanup;
}
pmk_buf_size = PSA_BITS_TO_BYTES(psa_get_key_bits(&key_attrs));
psa_reset_key_attributes(&key_attrs);
pmk_buf = (uint8_t *)os_malloc(pmk_buf_size);
if (!pmk_buf) {
wpa_printf(MSG_ERROR, "WPA3-PSA: Failed to allocate memory for PMK");
goto cleanup;
}
status = psa_export_key(shared_key_id, pmk_buf, pmk_buf_size, &pmk_len);
if (status != PSA_SUCCESS) {
wpa_printf(MSG_ERROR, "WPA3-PSA: Failed to export shared key (PMK): %d", status);
goto cleanup;
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +57 to +64
u8 password[256];
size_t password_len;
u8 shared_key[256]; /* Increased for PSA PAKE commit/confirm data */
size_t shared_key_len;
u8 local_commit[256]; /* Store local commit for context calculation */
size_t local_commit_len;
u8 peer_commit[256]; /* Store peer commit for context calculation */
size_t peer_commit_len;
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coded buffer sizes (256 bytes) are used throughout the code without clear justification. Consider defining named constants (e.g., WPA3_PSA_MAX_PASSWORD_LEN, WPA3_PSA_MAX_SHARED_KEY_LEN) to make the code more maintainable and document the rationale for these sizes.

Copilot uses AI. Check for mistakes.
}

/* SAE commit message should be exactly 98 bytes: [group][scalar][element] */
if (len != 98) {
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same hard-coded value 98 appears again for commit size validation. This duplication (also at line 349, 389, 1174) reinforces the need for a named constant to avoid maintenance issues.

Copilot uses AI. Check for mistakes.
Comment on lines +1030 to +1037
/* Use correct key type based on H2E */
if (params->h2e) {
psa_set_key_type(&attributes, PSA_KEY_TYPE_PASSWORD);
wpa_printf(MSG_DEBUG, "WPA3-PSA: Using PSA_KEY_TYPE_PASSWORD for H2E");
} else {
psa_set_key_type(&attributes, PSA_KEY_TYPE_PASSWORD);
wpa_printf(MSG_DEBUG, "WPA3-PSA: Using PSA_KEY_TYPE_PASSWORD for basic SAE");
}
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant code: The same key type PSA_KEY_TYPE_PASSWORD is set for both H2E and non-H2E cases. Either the conditional is unnecessary, or different key types should be used for different modes. If they're truly the same, remove the conditional and consolidate the logic.

Suggested change
/* Use correct key type based on H2E */
if (params->h2e) {
psa_set_key_type(&attributes, PSA_KEY_TYPE_PASSWORD);
wpa_printf(MSG_DEBUG, "WPA3-PSA: Using PSA_KEY_TYPE_PASSWORD for H2E");
} else {
psa_set_key_type(&attributes, PSA_KEY_TYPE_PASSWORD);
wpa_printf(MSG_DEBUG, "WPA3-PSA: Using PSA_KEY_TYPE_PASSWORD for basic SAE");
}
/* Use PSA_KEY_TYPE_PASSWORD for both H2E and basic SAE */
psa_set_key_type(&attributes, PSA_KEY_TYPE_PASSWORD);
wpa_printf(MSG_DEBUG, "WPA3-PSA: Using PSA_KEY_TYPE_PASSWORD for SAE (H2E: %s)", params->h2e ? "enabled" : "disabled");

Copilot uses AI. Check for mistakes.
/* Initialize operation fields */
op->state = SAE_NOTHING;
op->password_key = PSA_KEY_ID_NULL;
op->send_confirm = 1; /* Initialize send_confirm counter to 1 (not 0) */
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The send_confirm field is initialized to 1 with a comment stating "not 0", but this magic number needs clarification. Consider adding a more detailed comment explaining why the initial value must be 1 according to the SAE protocol specification.

Suggested change
op->send_confirm = 1; /* Initialize send_confirm counter to 1 (not 0) */
/*
* Initialize send_confirm counter to 1 (not 0).
* According to the SAE protocol specification (IEEE Std 802.11-2020, Section 12.4.5.4),
* the confirm message counter starts at 1 for the first confirm exchange.
* This ensures protocol compliance and prevents replay attacks.
*/
op->send_confirm = 1;

Copilot uses AI. Check for mistakes.
:local:
:depth: 2

Wi-Fi Direct® (also known as Wi-Fi P2P or peer-to-peer mode) enables direct device-to-device connections without requiring a traditional access point.
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Inconsistent hyphenation: The documentation uses "peer-to-peer" (with hyphens) in line 10 but "device-to-device" (also with hyphens) in the same sentence. While both are correct, consider using consistent terminology throughout. The industry standard term is "peer-to-peer" or "P2P".

Suggested change
Wi-Fi Direct® (also known as Wi-Fi P2P or peer-to-peer mode) enables direct device-to-device connections without requiring a traditional access point.
Wi-Fi Direct® (also known as Wi-Fi P2P or peer-to-peer mode) enables direct peer-to-peer connections without requiring a traditional access point.

Copilot uses AI. Check for mistakes.
@rado17 rado17 requested a review from richabp November 27, 2025 02:55
Copy link
Contributor

@FrancescoSer FrancescoSer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving this not to block rc1, however, I think the wifi direct page might need another in-depth docreview after the tagging.

@rlubos
Copy link
Contributor

rlubos commented Nov 28, 2025

Integrated in #25877

@rlubos rlubos closed this Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DNM doc-required PR must not be merged without tech writer approval. manifest manifest-nrfxlib manifest-zephyr

Projects

None yet

Development

Successfully merging this pull request may close these issues.